-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop support of php versions < 7.2 #132
Conversation
This reverts commit db0654f.
Hi @lutdev. Just a quick question here — the issue seemed more like a question (are we going to remove legacy PHP support?) rather than an issue or bug. I believe this module can work with PHP versions from say 7.0 to 8.3, so why wouldn't we allow that? It doesn't use modern constructs that wouldn't work on say 7.0, so we may as well just keep the support until we have to drop it for some reason. |
I agree with @michalkleiner. The php tracker here at least needs to support the PHP versions Matomo itself supports. Currently that would be PHP 7.2+ |
@michalkleiner yes, it was a question, for sure. I decided to drop the support 7.0 version because with it we can't use features that 8.1+ provides for us.
I don't want to do a lot of breaking changes in one PR :) That's why I didn't use features from 8.1+ Thanx @sgiehl for the information. It's a very strong argument for me. I suggest to change the minimal version to 7.2. What do you think about it? |
In that case we can increase the minimum required version in the same PR where we refactor to use modern 8.1+ features. Until then it is not needed to remove the 7.2 support.
7.2 minimum is fine with me. |
@michalkleiner perfect!) What are the next steps? What should I do? |
Let's reinstate the constant that you removed even if it's not directly used here, it would be a BC breaking change. You can also update changelog and I think the changes here should target |
@michalkleiner thank you!) I updated CHANGELOG.md |
@sgiehl this is now fine with me. Is it ok to merge this and tag |
@michalkleiner I updated CHANGELOG.MD and added information about dynamic properties |
@sgiehl just a friendly reminder about PR :) |
Description:
PR for #118